[codex] Update Spark Name registration payload format#10
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30c698b5dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| BOOST_CHECK_EQUAL(decodedData.sparkAddress, sparkNameData.sparkAddress); | ||
| BOOST_CHECK_EQUAL(decodedData.sparkNameValidityBlocks, sparkNameData.sparkNameValidityBlocks); | ||
| BOOST_CHECK_EQUAL(decodedData.additionalInfo, sparkNameData.additionalInfo); | ||
| BOOST_CHECK_EQUAL(decodedData.nVersion, spark::CSparkNameTxData::CURRENT_VERSION); |
There was a problem hiding this comment.
Avoid ODR-using CURRENT_VERSION in the test
When the spark_name_tests target is built, BOOST_CHECK_EQUAL binds both operands by reference, so passing CSparkNameTxData::CURRENT_VERSION here ODR-uses the header-only static const member. Because the class does not provide an out-of-class definition for that member and it is not inline constexpr, this added assertion can make the test binary fail to link with an undefined reference; compare against a cast/local value or make the constant inline/constexpr.
Useful? React with 👍 / 👎.
What changed
CSparkNameTxData::CURRENT_VERSIONfrom 1 to 2.operationTypebyte, defaulting to0for registration.Why
Firo's Spark Name registration format now serializes registration data as v2 and includes an operation type. Stack Wallet only creates registrations, so this intentionally keeps transfer fields out of the mobile library.
Review notes
This is the smallest native change needed for registration compatibility. Transfer support can be added later if mobile starts creating Spark Name transfers.
Validation
git diff --checkpasses locally, with only Windows CRLF warnings.spark_name_testsfrom a temp clone with MSYS2/UCRT; secp256k1 built, then the standalone compile stopped on missingendian.hbecause this repo's bare test target assumes POSIX/Linux headers.firoorg/flutter_libsparkmobile's Windows CMake build and linkedlibflutter_libsparkmobile.dll.